Skip to content

Conversation

@stevenhua0320
Copy link

@stevenhua0320 stevenhua0320 commented Nov 17, 2025

@sbillinge ready for review, we should test this whether it would be successful to build CI on macos14.

@codecov
Copy link

codecov bot commented Nov 17, 2025

Welcome to Codecov 🎉

Once you merge this PR into your default branch, you're all set! Codecov will compare coverage reports and display results in all future pull requests.

Thanks for integrating Codecov - We've got you covered ☂️

Copy link
Contributor

@sbillinge sbillinge left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it better to do this or to add gsl to the tests.txt requirements (or the conda requirements)?

@stevenhua0320
Copy link
Author

Yes, it might be a better way.

Copy link
Contributor

@sbillinge sbillinge left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We didn't need it in both places. Just tests unless it is needed to run the code itself, in which case, just in conda.txt and pip.txt

@sbillinge
Copy link
Contributor

@stevenhua0320 I looked more into this and it will be a runtime dependency, so we need to add it to the conda.txt and pip.txt files and can then remove it from the tests.txt. We also likely need io add it to the host: and run: section of the meta.yaml in the conda-forge feedstock.

Actually, looking at diffpy.pdffit2 it seems that this was addressed (and tests are passing there) in diffpy/diffpy.pdffit2#144 . There gsl was added to condat.txt requirements in pdffit2. I wonder if the issue is that we didn't add it to the meta.yaml of pdffit2-feedstock so when the dependency installs it from conda-forge it doesn't come with gsl. Is this possible?

tl;dr: Let's try a fix where we see if adding gsl as a dependency to the meta.yaml in pdffit2 fixes the problem over here?

@stevenhua0320
Copy link
Author

@sbillinge I have checked the feedstock in diffpy.pdffit2 and I saw @Tieqiong has added gsl into both the requirements in the package and the feedstock. So, the problem does come from diffpy.distanceprinter itself. So, add gsl as an requirement to both this package and the feedstock should solve the problem. I would make the commit shortly.

@sbillinge
Copy link
Contributor

@sbillinge I have checked the feedstock in diffpy.pdffit2 and I saw @Tieqiong has added gsl into both the requirements in the package and the feedstock. So, the problem does come from diffpy.distanceprinter itself. So, add gsl as an requirement to both this package and the feedstock should solve the problem. I would make the commit shortly.

Something is not right here. Gsl is not a requirement of distanceprinter but it is a requirement of pdffit2. It would be good to get to the bottom of why it is not getting loaded as part of the pdffit2 requirements

@stevenhua0320
Copy link
Author

It is because in macos14 the homebrew removed many bottles, including the one for gsl. So, if we did not pre-install the gsl bottles, we cannot run the diffpy.pdffit2 on a clean macos14 environment. As diffpy.distanceprinter needs to run diffpy.pdffit2 function, we need gsl install in the env so that it could be run.

@sbillinge
Copy link
Contributor

It is because in macos14 the homebrew removed many bottles, including the one for gsl. So, if we did not pre-install the gsl bottles, we cannot run the diffpy.pdffit2 on a clean macos14 environment. As diffpy.distanceprinter needs to run diffpy.pdffit2 function, we need gsl install in the env so that it could be run.

if gsl is a dependency of pdffit2, then it should be installed already. This is my point.

@stevenhua0320
Copy link
Author

I'm afraid it is not defined as a dependency but as build requirement in diffpy.pdffit2? The feedstock looks like this in diffpy.pdffit2

requirements:
  build:
    - {{ compiler('c') }}
    - {{ compiler('cxx') }}
    - {{ stdlib("c") }}
    - gsl
    - python                              # [build_platform != target_platform]
    - cross-python_{{ target_platform }}  # [build_platform != target_platform]
  host:
    - python
    - pip
    - setuptools
    - setuptools-git-versioning >=2.0
    - gsl
  run:
    - python
    - setuptools
    - diffpy.structure

@sbillinge
Copy link
Contributor

Let's try adding it to run and we will see if it allows the tests to pass for distanceprinter

@stevenhua0320
Copy link
Author

I think I have added in the feedstock in the diffpy.distanceprinter in the morning. It's ready for review.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants